Skip to content

Conversation

@mtoffl01
Copy link
Contributor

@mtoffl01 mtoffl01 commented Oct 22, 2025

What does this PR do?

This is a POC of the "Go Config Revamp" detailed here.

Creates a new internal/config package which initializes a Config singleton once, on startup. The fields on this Config type are populated from the following sources, in order of priority: Managed Declarative Config, Datadog Env vars, Otel Env vars, and local Declarative Config. Defaults are defined as a fallback when no source has been configured for the field.

Config has been populated with a few starter fields and a few starter getters, with more to come as tracer, profiler, and other products migrate over to using the internal config as their source of truth for configuration. A single configuration DD_TRACE_DEBUG has been migrated over from tracer to internal/config, which proves that the concept works.

Motivation

See: Problem Statement
Configuration was disorganized and spread out across the repo.

Reviewer tips

Reviewers should focus on the config.go and configprovider.go files, as this is new code.

The ConfigSource implementations have been pulled from preexisting code in dd-trace-go. So, you can review these files with a little less scrutiny:

  • otelenvconfigsource pulled from ddtrace/tracer/otel_dd_mappings.go, but the corresponding tests are new
  • declarativeconfigsource and its tests were pulled from the stableconfig package
  • envconfigsource is quite straightforward

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@pr-commenter
Copy link

pr-commenter bot commented Oct 22, 2025

Benchmarks

Benchmark execution time: 2025-11-13 20:43:15

Comparing candidate commit 9e109e8 in PR branch mtoff/config-revamp with baseline commit 9db3a3d in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 8 metrics, 0 unstable metrics.

@datadog-official
Copy link
Contributor

datadog-official bot commented Oct 24, 2025

⚠️ Tests

⚠️ Warnings

❄️ 2 New flaky tests detected

TestWriter_Flush_MultipleEndpoints from github.com/DataDog/dd-trace-go/v2/internal/telemetry/internal (Datadog)
Failed

=== RUN   TestWriter_Flush_MultipleEndpoints
    writer_test.go:242: 
        	Error Trace:	/home/runner/work/dd-trace-go/dd-trace-go/internal/telemetry/internal/writer_test.go:242
        	Error:      	Should not be zero, but was 0
        	Test:       	TestWriter_Flush_MultipleEndpoints
==================
Read at 0x00c0001121f8 by goroutine 23:
  github.com/DataDog/dd-trace-go/v2/internal/telemetry/internal.TestWriter_Flush_MultipleEndpoints()
...
TestWriter_Flush_Success from github.com/DataDog/dd-trace-go/v2/internal/telemetry/internal (Datadog)
Failed

=== RUN   TestWriter_Flush_Success
==================
Read at 0x00c00038018f by goroutine 45:
  github.com/DataDog/dd-trace-go/v2/internal/telemetry/internal.TestWriter_Flush_Success()
      /home/runner/work/dd-trace-go/dd-trace-go/internal/telemetry/internal/writer_test.go:138 +0x4f1
  testing.tRunner()
      /opt/hostedtoolcache/go/1.24.10/x64/src/testing/testing.go:1792 +0x225
  testing.(*T).Run.gowrap1()
...

ℹ️ Info

🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 9e109e8 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants